Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix whitespace not matching in since command. #778

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

joemcgill
Copy link
Member

Summary

This fixes an issue where the since command was failing to replace instances of n.e.x.t when more than once space occurred between the @since and the placeholder version number, e.g., for docblock alignment purposes.

Relevant technical choices

While prepping for the 2.5.0 release, I noticed that running npm run since -- -r 2.5.0 missed one case in modules/images/fetchpriority/can-load.php that was added during this release. It looks like this is due to the alignment spacing in the docblock, which is causing the regex pattern to not match. Updating the regex pattern to look for multiple spaces and apply the same number of spaces in the replacement fixes the issue.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

This fixes an issue where the since command was failing to replace instances of `n.e.x.t` when more than once space occurred between the `@since` and the placeholder version number, e.g., for docblock alignment purposes.
@joemcgill joemcgill added [Type] Bug An existing feature is broken Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release labels Jul 13, 2023
@joemcgill joemcgill self-assigned this Jul 13, 2023
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joemcgill Good catch, LGTM!

@felixarntz
Copy link
Member

@joemcgill Maybe adjust the base branch to release/2.5.0 so it already applies in that one?

@mukeshpanchal27 mukeshpanchal27 changed the base branch from trunk to release/2.5.0 July 14, 2023 04:04
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joemcgill for the PR. LGTM.

  • I have updated the base branch to release/2.5.0.
  • In 08d5176 we have similar string with spaces for since annotation in modules/images/webp-uploads/can-load.php file but it working fine for that release don't know what has changed 🤔

@joemcgill
Copy link
Member Author

Thanks for adjusting the branch, @mukeshpanchal27. I noticed the other place where this would have applied and thought the same thing! I assume that one just got adjusted manually.

@joemcgill joemcgill merged commit e7b5750 into release/2.5.0 Jul 14, 2023
16 checks passed
@joemcgill joemcgill deleted the fix/since-command-whitespace branch July 14, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Bug An existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants